Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support Dust tree by age #413

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

wugeer
Copy link
Contributor

@wugeer wugeer commented Jul 12, 2024

Background

Dust tree by age #385
Add a new feature to dust to show the tree by age

Full Changelogs

  • feat: support Dust tree by age

Issue Reference

fix: Dust tree by age #385

Test Result

dust help command output
image
cargo test run test results

image
image
image
image

show by accesstime
image
show by modifiedtime
image
show by createdtime
image


// 按照指定格式格式化为字符串
local_datetime.format("%Y%m%dT%H:%M:%S").to_string()
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe: "%Y-%m-%dT%H:%M:%S"

Also lets get rid of the comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will resubmit the code after testing the performance and fix this together

Copy link
Owner

@bootandy bootandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick scan says it looks sensible. I am a bit worried about performance though

I created: #415 to track down the performance issues with a similar PR.

@wugeer
Copy link
Contributor Author

wugeer commented Jul 17, 2024

A quick scan says it looks sensible. I am a bit worried about performance though

I created: #415 to track down the performance issues with a similar PR.

I will test my submitted performance test, if the test is OK, I will notify you to help me review the code.

@wugeer wugeer force-pushed the feature_show_by_mtime branch from 300565c to 8770645 Compare July 19, 2024 14:10
@wugeer
Copy link
Contributor Author

wugeer commented Jul 19, 2024

A quick scan says it looks sensible. I am a bit worried about performance though

I created: #415 to track down the performance issues with a similar PR.

Hello, Tests with hyperfine show that this PR means we are running ~ 4% slower than the v1.0.0 release. I'm not sure if the performance test results are okay. Here are my test results.
image
image
image
image
I'm not sure if my performance tests are sufficient. If more information is needed, please let me know, and I will provide it as soon as possible. :) @bootandy

@wugeer wugeer force-pushed the feature_show_by_mtime branch from 8770645 to 3ee3c16 Compare July 19, 2024 14:25
@bootandy
Copy link
Owner

I think this is sensible, I just want to sleep on it for a bit.

@bootandy
Copy link
Owner

Hello, Tests with hyperfine show that this PR means we are running ~ 4% slower than the v1.0.0 release. I'm not sure if the performance test results are okay. Here are my test results.

My hyperfine tests show this branch is about 4% faster !

Either way its fine, I'm looking for drops of >10% before I get nervous.

@bootandy
Copy link
Owner

Why did you choose the flag -m ? It's fine but I'd like to copy an existing tool that uses a similar flag if possible

@wugeer

@wugeer
Copy link
Contributor Author

wugeer commented Aug 1, 2024

Why did you choose the flag -m ? It's fine but I'd like to copy an existing tool that uses a similar flag if possible This is fine, but if possible I'd like to copy an existing tool that uses a similar flag

@wugeer

hello, I chose randomly from the remaining single-letter parameters. I originally wanted to use -t but it was already taken. Do you have any other good suggestions? @bootandy

@bootandy
Copy link
Owner

bootandy commented Aug 2, 2024

I don't have other suggestions, I'll research some other tools to see if anyone has anything similar, but we'll go with -m if I can't find anything.

@bootandy bootandy merged commit f48fcc7 into bootandy:master Aug 7, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants